fix(recipes): nvsentinel v1.22.0 + re-enable VR200 metadata-collector - #2711

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

No actionable comments were generated in the recent review. 🎉

Merge Risk: 🔵 Low · up to ea9a8

The new test may pass while metadata collection remains disabled; add the missing enablement assertion before relying on this regression coverage.

_{Comment @coderabbitai help to get the list of available commands.}

@framsouza this PR now has merge conflicts with main. Please rebase to resolve them.

Recipe evidence check

Registry change: scoped to recipes that reference a changed component
entry in recipes/registry.yaml (not every leaf).

Protected recipes

Recipes with committed evidence (recipes/evidence/<slug>/<source>/<digest>.yaml) that this PR affects: 18

How to refresh evidence

Run on a cluster matching the recipe's criteria:

aicr snapshot -o snapshot.yaml
# Profiled families (AKS/GKE gpuStack): hydrate the recipe with the
# pointer's recorded 'profile:' selection first — validating the raw
# overlay resolves only the declaration default, and 'aicr validate'
# has no --profile flag. AKS additionally needs the pool projection
# (GKE uses the plain snapshot above):
#   az aks nodepool list -g <rg> --cluster-name <cluster> -o json > pools.json
#   aicr snapshot --aks-gpu-pools pools.json -o snapshot.yaml
#   aicr recipe -s snapshot.yaml --intent <intent> [--platform <platform>] \
#     --profile <name>=<value> -o recipe.yaml
# State the target leaf's intent/platform explicitly (the snapshot
# fingerprint supplies service/accelerator/OS but intent and platform
# default to 'any') and pass -r recipe.yaml below instead of the raw
# overlay.
aicr validate \
  -r recipes/overlays/<slug>.yaml \
  -s snapshot.yaml \
  --emit-attestation ./out \
  --push ghcr.io/<your-fork>/aicr-evidence
# Copy to the per-source path printed in the emit 'copyTo' hint:
#   recipes/evidence/<slug>/<source>/<bundle-digest>.yaml

This gate is warning-only and never blocks merge. See ADR-007 for the trust model.

🌿 Preview your docs: https://nvidia-preview-fix-nvsentinel-v1-22-0-vr200-metadata-collector.docs.buildwithfern.com/aicr

Squashed a doc fix into the version-bump commit: docs/user/component-catalog.md still said the disabled-component list was verified against v1.20.0; bumped to v1.22.0.

• Old HEAD: ea9a8d36
• New HEAD: 7a575c31

Verification against the pulled oci://ghcr.io/nvidia/nvsentinel:v1.22.0 chart:

• All 4 on-by-default subcharts in the doc (gpuHealthMonitor, syslogHealthMonitor, metadataCollector, labeler) match values.yaml enabled: true.
• All 6 primary off-by-default components (mongodbStore, faultQuarantine, nodeDrainer, faultRemediation, janitor, janitorProvider) match enabled: false.
• All 10 "also off" components (healthEventsAnalyzer, lifecycleManager, cspHealthMonitor, kubernetesObjectMonitor, nicHealthMonitor, slurmDrainMonitor, preflight, eventExporter, inclusterFileServer, k8sdatastoreCrds) match enabled: false.
• No components added or removed between v1.20.0 and v1.22.0 (Chart.yaml dependency set unchanged). The two new CRDs in v1.22.0 (maintenancerequests.nvsentinel.dgxc.nvidia.com, perconaservermongodbclustersyncs.psmdb.percona.com) live inside the already-listed lifecycleManager and mongodbStore subcharts.
• metadata-collector/values.yaml:35 docstring pointer in pkg/bundler/validations/checks.go re-verified — runtimeClassName: "nvidia" is still on line 35.

make check-docs-mdx-parse OK. Both commits retain GPG signatures and DCO sign-offs through the fixup rebase.

@framsouza looks like golden is out of sync, resolve these conflicts and we can merge this.

Rebased onto main (b4d7a3c8) to clear the golden conflict.

• Old HEAD: 4cb9acab
• New HEAD: 30d1f9cb (still single commit)

Only pkg/bundler/testdata/stock_render_golden.yaml conflicted this round. Resolved with --ours (upstream baseline) + make update-goldens so the nvsentinel v1.22.0 pin re-layers on the fresh tree. make bom-docs produced no drift. TestStockRenderGolden + TestCatalogParity pass. Still GPG-signed, DCO sign-off preserved.

@framsouza this PR now has merge conflicts with main. Please rebase to resolve them.

Rebased onto main (938716c0) to clear the golden conflict.

• Old HEAD: fe012650 (dropped 1 clean "Update branch" merge commit — empty combined diff)
• New HEAD: 830638de (still single commit)

Only pkg/bundler/testdata/stock_render_golden.yaml conflicted this round. Resolved with --ours (upstream baseline) + make update-goldens so the nvsentinel v1.22.0 pin re-layers on the fresh tree. make bom-docs picked up a small busybox:1.38.0 digest drift from upstream chart templates (gpu-operator + network-operator sections); folded into the same commit via amend. TestStockRenderGolden + TestCatalogParity pass. Still GPG-signed, DCO sign-off preserved.

@framsouza looks like NVS 1.25 is out
https://github.com/NVIDIA/NVSentinel/releases/tag/v1.25.0

Heads-up from this week's component drift review, not a review comment on the
change itself.

nvsentinel upstream has moved while this has been open. The pin here is
v1.22.0; upstream is now v1.25.0 (released 2026-09-28 09:41 UTC), so this
targets three releases back.

Worth knowing about the cadence before deciding: nvsentinel has shipped one minor
per week since v1.18.0, with no gaps.

That cadence is also why the weekly drift digest is not a good source for this
component - this morning's report (07:35 UTC) proposed v1.24.0 and was already
stale two hours later.

No opinion from me on whether to move this PR's target. If the VR200
metadata-collector work was validated against v1.22.0, re-validating against
v1.25.0 may cost more than it is worth, and bumping separately afterwards is
perfectly reasonable. Flagging it mainly because this PR gates #2876 (VR200
evidence at v1.0.0-rc1), so whichever version lands here is the one that
release-candidate evidence gets captured against.

Rebased onto main (6c73d516) and addressed the open threads.

• Old HEAD: 1689428f
• New HEAD: 7a90d110 (two commits: the rebased bump, plus one appended commit for the review feedback)

Force-pushed because the rebase was gate-required; inline comment anchors on 1689428f are outdated as a result.

Conflicts resolved this round

• pkg/bundler/validations/nvsentinel_nic_health_monitor_render_test.go — main-side moved nicHealthMonitorInitImage off the frozen bitnamilegacy/os-shell default onto the digest-pinned busybox:1.38.0, and added the doc comment explaining why asserting it also pins the override. Kept main's side of both; re-applied only the v1.20.0 → v1.22.0 bump on nicHealthMonitorImage.
• pkg/bundler/testdata/stock_render_golden.yaml and docs/user/container-images.md — resolved --ours (upstream baseline), then re-layered via make update-goldens and make bom-docs. pkg/recipe/testdata/catalog_parity_golden.yaml merged cleanly.

Review threads

@mchmarny, "MAJOR — Update the coupled NVSentinel image pins with this chart bump" — done, in the appended commit. Three surfaces carry the nvsentinel version by hand and none of them had moved:

• .github/workflows/vuln-scan-images.yaml — the six mixin-gated nvsentinel images in the scan matrix include: overrides (kubernetes-object-monitor, preflight, preflight-dcgm-diag, preflight-nccl-loopback, preflight-nccl-allreduce, nic-health-monitor). docker.io/library/busybox deliberately left alone — it comes from global.initContainerImage, not the chart's appVersion, so it does not move with defaultVersion.
• tools/mirror-e2e — PREFLIGHT_CONTROLLER_IMAGE, the four PREFLIGHT_IMAGES, OBJECT_MONITOR_IMAGE, and the subchart entry in NIC_HEALTH_MONITOR_IMAGES (its init-container entry left alone, same reason).
• docs/user/container-images.md — the two mixin-image references in the hand-maintained prose section below the generated block.

On the drift-report expectation: the inconsistency is in tools/drift-report/main_test.go's runFixtureReport, whose own doc comment requires currentValue to track the live registry pin — a drifted fixture routes that dep to the mismatch branch and surfaces as an off-by-one in Summary.Unresolved far from the fixture line. Bumped to v1.22.0. The v1.20.0 strings in renovate_test.go, report_test.go and slack_test.go are deliberately left: those are pure fixture-parsing tests that never read the real registry (only main_test.go, annotations_test.go and config_test.go use testRepoRoot), so their values are arbitrary.

TestObjectMonitorImagePinnedEverywhere, TestPreflightImagesPinnedEverywhere and TestNICHealthMonitorImagePinnedEverywhere are all registry-driven, so they now pass against the live pin rather than a hand-copied one.

@coderabbitai, "Assert that metadata-collector.enabled is true" — the concern is real, the suggested key is not. Enablement lives at global.metadataCollector.enabled (the chart's subchart toggle), not metadata-collector.enabled; this PR removes the false override and lets the chart default (on) apply, so there is no enabled key inside the metadata-collector block to read. Fixed the underlying vacuity instead: TestVR200NVSentinelNRIHostMountReenable now fails if a leaf re-adds global.metadataCollector.enabled: false. Absent stays the wanted state — only an explicit false is a regression, so the assertion does not break when the chart default is relied on.

@mchmarny, "MINOR — Update the documented NVSentinel defaults" — this was addressed in the 2026-09-16 round (docs/user/component-catalog.md now says verified against v1.22.0); carried through the rebase unchanged.

Also in this round

recipes/overlays/h200-k0s-ubuntu-training.yaml — its metadataCollector.enabled: false comment said "Same pairing as the vr200-rke2-ubuntu leaves". That pairing no longer exists after this change, so the pointer now says what actually happened instead of asserting a shape that is gone. The h200-k0s leaf itself is unchanged — its own reason for disabling (node-selector / 0-desired DaemonSet) is separate, and whether the host-mount shape applies there is not something this PR validated.

Verification

golangci-lint -c .golangci.yaml run ./pkg/recipe/... ./pkg/bundler/... ./tools/drift-report/...   # 0 issues
go test -short -race -count=1 ./pkg/recipe/... ./pkg/bundler/... ./tools/drift-report/...          # all ok
make lint-yaml license check-agents-sync check-docs-filenames check-docs-mdx \
     check-docs-mdx-parse check-docs-yaml bom-pinning-check check-depproxy-kit \
     check-upgrade-records                                                                         # all OK
make coverage-check    # docs/user/coverage-matrix.md up to date
make tuning-check      # docs/integrator/components/nodewright.md up to date
make update-goldens / make bom-docs   # regenerated, committed

Go changes this round are test-only plus a fixture constant, so no coverage delta. Still deferred to CI: kwok-e2e, api-diff/openapi-diff (no SDK or REST surface change), tools/e2e, make scan.

On v1.25.0

Staying at v1.22.0. The VR200 host-mount work, the CRD-ownership re-audit and the value-drift audit in the description were all done against v1.22.0, and re-targeting means redoing all three plus the BOM and goldens. Given the one-minor-per-week cadence you documented, chasing the tip inside this PR means it is stale again before it merges. Happy to open the bump as a separate PR once this lands — that also keeps #2876's RC evidence tied to a version whose audit is written down here rather than one audited in a rush.

One thing worth a maintainer call, not resolved here: nvsentinel has no ADR-021 transition record (recipes/components/nvsentinel/upgrades.yaml does not exist, and the registry entry has no upgrades.file), so check-upgrade-records does not cover this bump. I did not author one — no UAT or KWOK lane exercises an nvsentinel upgrade, so there is nothing to put in verifiedBy for a safe verdict, and per the authoring guide an unwritten record reporting unknown beats a safe that is not backed. Flagging it in case you want it as a follow-up.

#!/bin/bash
set -euo pipefail
git rev-parse --short HEAD
ast-grep outline pkg/recipe/nvsentinel_vr200_hostmount_test.go --match TestVR200NVSentinelNRIHostMountReenable --view expanded
rg -n -C 6 'metadataCollector|metadata-collector|enabled' pkg/recipe/nvsentinel_vr200_hostmount_test.go recipes/overlays/vr200-rke2-ubuntu-training.yaml recipes/overlays/vr200-rke2-ubuntu-inference.yaml
git status --short

@framsouza, verification succeeded at HEAD 7a90d110. TestVR200NVSentinelNRIHostMountReenable now fails when global.metadataCollector.enabled is explicitly false. Both VR200 overlays omit that override and rely on the chart default.

I was wrong to say verification was unavailable in my previous reply. The original test-coverage concern is addressed; my suggested metadata-collector.enabled key was incorrect. This confirms the test change, not VR200/RKE2 hardware behavior, which remains pending.

You are interacting with an AI system.